Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

get costs_year.csv instead of costs.csv #1120

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

get costs_year.csv instead of costs.csv #1120

wants to merge 7 commits into from
+3 −15

Conversation

danielelerede-oet
Copy link
Contributor

Changes proposed in this Pull Request

As the column year has been dropped from resources/costs.csv this PR proposes a similar approach to pypsa-eur to retrieve a costs.csv file also reporting the reference year for the relative techno-economic assumptions (as from the costs section in the config file).

Checklist

  • I consent to the release of this PR's code under the AGPLv3 license and non-code contributions under CC0-1.0 and CC-BY-4.0.
  • I tested my contribution locally and it seems to work fine.
  • Code and workflow changes are sufficiently documented.
  • Newly introduced dependencies are added to envs/environment.yaml and doc/requirements.txt.
  • Changes in configuration options are added in all of config.default.yaml and config.tutorial.yaml.
  • Add a test config or line additions to test/ (note tests are changing the config.tutorial.yaml)
  • Changes in configuration options are also documented in doc/configtables/*.csv and line references are adjusted in doc/configuration.rst and doc/tutorial.rst.
  • A note for the release notes doc/release_notes.rst is amended in the format of previous release notes, including reference to the requested PR.

@danielelerede-oet danielelerede-oet changed the title Costs year get costs_year.csv instead of costs.csv in resources Sep 27, 2024
@davide-f
Copy link
Member

Great @danielelerede-oet :D
This seems nearly done; probably we can slightly improve it further.
In particular, here:

pypsa-earth/Snakefile

Lines 393 to 420 in 3142b1f

rule retrieve_cost_data:
input:
HTTP.remote(
f"raw.githubusercontent.com/PyPSA/technology-data/{config['costs']['version']}/outputs/costs_{config['costs']['year']}.csv",
keep_local=True,
),
output:
COSTS,
log:
"logs/" + RDIR + "retrieve_cost_data.log",
resources:
mem_mb=5000,
run:
move(input[0], output[0])
rule retrieve_cost_data_flexible:
input:
HTTP.remote(
f"raw.githubusercontent.com/PyPSA/technology-data/{config['costs']['version']}/outputs/costs"
+ "_{planning_horizons}.csv",
keep_local=True,
),
output:
costs=COSTDIR + "costs_{planning_horizons}.csv",
resources:
mem_mb=5000,
run:
move(input[0], output[0])

we probably don't need anymore two versions of the same rule where only the output file path is changed.
with the revision of costs with "costs_{year}", the retrieve_cost_data_flexible should match the format of costs.
I'd propose to delete the retrieve_cost_data rule and rename retrieve_cost_data_flexible into retrieve_cost_data.

That should clean the code too.
Great contribution and don't forget a line in release note :)

@danielelerede-oet
Copy link
Contributor Author

Ciao @davide-f . Considering that in the config file the argument to be passed to costs is year and not planning_horizons, I think we should just leave the current retrieve_cost_data instead of using retrieve_cost_data_flexible and renaming it to retrieve_cost_data. Is that right or am I missing anything? :)

@danielelerede-oet danielelerede-oet changed the title get costs_year.csv instead of costs.csv in resources get costs_year.csv instead of costs.csv Oct 3, 2024
@danielelerede-oet
Copy link
Contributor Author

Hi @davide-f, I perfomed some tests and actually keeping retrieve_cost_data_flexible only and renaming it to retrieve_cost_data is not a viable solution: in that case resources/costs_{year}.csv is not generated at all. Using planning_horizons as wildcard always results in a KeyError. The only working solution is the one I proposed in the latest commit, but I'm open to discuss it :)

@davide-f
Copy link
Member

davide-f commented Oct 29, 2024 •
edited
Loading

As we are addressing the retrieve_cost_data, it would be nice to add the "params:" snakemake option into the rule as:

params:
  version: config["costs"]["version"]

And update the default tech_data version to the latest: 0.9.2

pypsa-earth/config.default.yaml

Line 358 in 8d93ddb

version: v0.6.2

with version: 0.9.2

This ensures that if the tech_data version is changed, the rule is retriggered

This solves: #1073

Otherwise we can address this in a new version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@danielelerede-oet @davide-f